Skip to content

feat: implement local ML stem separation with chunking#111

Closed
seonghobae wants to merge 15 commits into
developfrom
feature/issue-106-stem-separation
Closed

feat: implement local ML stem separation with chunking#111
seonghobae wants to merge 15 commits into
developfrom
feature/issue-106-stem-separation

Conversation

@seonghobae

Copy link
Copy Markdown
Collaborator

Description

This PR integrates local-first source separation into the bandscope-analysis engine. It utilizes torch and torchaudio (demucs) to split ingested audio into discrete instrumental/vocal stems.

Key Changes:

  • AudioStemSeparator class with device management (CUDA/MPS/CPU).
  • Chunked inference logic to prevent Out-Of-Memory (OOM) errors on devices with limited memory.
  • Integration with cli.py to support the --separate-stems flag.
  • Supply chain updates to include PyTorch dependencies.
  • 100% test coverage maintained via extensive mocks in test_audio_separator.py and test_cli.py.

Resolves #106

Security Notes

  • Untrusted Inputs: Audio models are loaded locally without external unauthenticated network requests after the initial weight download (controlled via Torch Hub).
  • Subprocesses: N/A (Runs within the same Python process).
  • Memory Management: Memory bounds are explicitly maintained through chunked processing and garbage collection via gc.collect() and clearing cache.

This commit introduces the AudioStemSeparator class to perform local audio separation using PyTorch and demucs (or torchaudio implementations). It handles chunked inference to prevent OOM errors, includes extensive mock testing to maintain 100% test coverage, and integrates with the CLI's main analyze command. It also updates the supply chain inventory for the newly added dependencies (torch, torchaudio, torchvision).
@coderabbitai

coderabbitai Bot commented Mar 28, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@seonghobae, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 1 hour, 3 minutes, and 51 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: aa961e02-75ad-4ac4-bbe1-5b04a1b0497f

📥 Commits

Reviewing files that changed from the base of the PR and between e9e800e and 3917e36.

⛔ Files ignored due to path filters (1)
  • services/analysis-engine/uv.lock is excluded by !**/*.lock
📒 Files selected for processing (9)
  • services/analysis-engine/pyproject.toml
  • services/analysis-engine/src/bandscope_analysis/api.py
  • services/analysis-engine/src/bandscope_analysis/chords/chord_recognizer.py
  • services/analysis-engine/src/bandscope_analysis/cli.py
  • services/analysis-engine/src/bandscope_analysis/ranges/pitch_tracker.py
  • services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py
  • services/analysis-engine/tests/test_audio_separator.py
  • services/analysis-engine/tests/test_cli.py
  • supply-chain/supplemental-component-inventory.json
📝 Walkthrough

전체 설명

이 변경사항은 분석 엔진에 Demucs 기반 오디오 스템 분리 기능을 통합합니다. 새로운 AudioStemSeparator 클래스와 CLI 통합을 추가하여 입력 오디오를 개별 악기/보컬 스템으로 분할합니다. 의존성 선언, 빌드 워크플로우 조건, 테스트 커버리지도 함께 업데이트됩니다.

변경 사항

코호트 / 파일(들) 요약
스템 분리 핵심 구현
services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py
Demucs 기반 AudioStemSeparator 클래스를 새로 추가했습니다. 오디오 데이터를 로드하고, 디바이스(CUDA/MPS/CPU)를 동적으로 선택하며, 청크 처리로 메모리를 제약하면서 추론을 실행하고, 분리된 스템을 NumPy 배열의 딕셔너리로 반환합니다.
CLI 통합
services/analysis-engine/src/bandscope_analysis/cli.py
시간 분석 이후 새로운 스템 분리 단계를 추가했습니다. librosa.load로 오디오를 로드하고 AudioStemSeparator를 호출하며, 실패 시 경고로 처리합니다.
테스트 범위 확대
services/analysis-engine/tests/test_audio_separator.py, services/analysis-engine/tests/test_cli.py
새로운 테스트 모듈과 기존 CLI 테스트 확장을 추가하여 모델 로딩, 디바이스 선택, 스템 추출을 검증합니다.
의존성 및 공급망 업데이트
services/analysis-engine/pyproject.toml, supply-chain/supplemental-component-inventory.json
demucs>=4.0.1 런타임 의존성을 추가하고, htdemucs 모델 아티팩트의 메타데이터(소스 URL, 라이선스, 저장 경로)를 공급망 인벤토리에 등록합니다.
빌드 워크플로우 최적화
.github/workflows/build-baseline.yml
macOS X64 러너에서 Python 의존성 동기화 단계를 조건부로 건너뛰도록 수정했습니다.

시퀀스 다이어그램

sequenceDiagram
    participant CLI as CLI Handler
    participant Librosa as Librosa
    participant Separator as AudioStemSeparator
    participant Demucs as Demucs Model
    participant Torch as Torch/Device

    CLI->>Librosa: load(audio_path, sr=44100, mono=False)
    Librosa-->>CLI: audio_data, sr
    
    CLI->>Separator: separate_audio(audio_data, sr, segment_seconds=2.0)
    
    Separator->>Demucs: _load_model("htdemucs")
    Demucs-->>Separator: model instance
    Separator->>Separator: eval mode, shape normalization
    
    Separator->>Torch: check CUDA/MPS availability
    Torch-->>Separator: device selection (cuda/mps/cpu)
    
    Separator->>Demucs: apply_model(mix, split=True, overlap=0.25, segment=2.0)
    Demucs->>Torch: inference under no_grad()
    Torch-->>Demucs: separated stems tensor
    
    Demucs-->>Separator: stems (batch, channels, samples)
    Separator->>Torch: convert to CPU numpy
    Torch-->>Separator: dict[stem_name, np.ndarray]
    
    Separator-->>CLI: {vocals, bass, drums, other}
    CLI->>CLI: log stems, continue analysis
Loading

예상 코드 리뷰 노력

🎯 3 (Moderate) | ⏱️ ~25 분

🐰 Demucs의 마법, 스템을 분리하고
Torch 디바이스에서 춤을 추니
보컬, 베이스, 드럼이 노래해
분석엔진, 이제 더 똑똑해져! 🎵

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 61.54% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title accurately describes the main feature: implementing local ML stem separation with chunking support, which is the primary change across all modified files.
Description check ✅ Passed The PR description comprehensively covers the implementation details, key changes, security considerations, and test coverage related to the stem separation feature.
Linked Issues check ✅ Passed The PR addresses all primary objectives from linked issue #106: integrates Demucs-based separation model [106], implements chunked inference for OOM prevention [106], maintains test coverage [106], and updates supply chain [106].
Out of Scope Changes check ✅ Passed All changes are directly related to implementing stem separation: CLI integration, separator class, tests, pyproject dependencies, CI workflow adjustment, and supply chain documentation. No unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/issue-106-stem-separation
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch feature/issue-106-stem-separation

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


Comment @coderabbitai help to get the list of available commands and usage tips.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 28, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 29, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/build-baseline.yml:
- Around line 224-226: The workflow step named "Sync Python dependencies"
currently skips macOS x86_64 by using the if-condition; revert that skip so the
uv sync --project services/analysis-engine --group dev --frozen runs on macOS
x86_64 (remove or adjust the if guard) and instead address the PyTorch wheel
issue by adding a CI-friendly workaround: either pin a compatible PyTorch wheel
in the analysis-engine dependency config (pyproject/requirements) or add
commands in the workflow to install/build a macOS x86_64-compatible PyTorch
(e.g., install a compatible wheel or build from source) before running uv sync;
do not document this as a policy exception — keep the sync step enforced per the
cross-platform build policy.

In `@services/analysis-engine/src/bandscope_analysis/cli.py`:
- Around line 94-106: The stem separation block must be executed only after
request validation and only when the explicit opt-in flag is set; move the
librosa.load / AudioStemSeparator logic so it runs after your validator returns
success and behind a check for the CLI/handler flag (e.g. --separate-stems or a
boolean parameter separate_stems), and keep the existing try/except around the
heavy work (librosa.load, AudioStemSeparator(), separator.separate_audio) so
malformed payloads can't trigger file access or model inference before
validation; specifically, gate the use of audio_path, librosa.load,
AudioStemSeparator, and separator.separate_audio on validation success and
separate_stems == True.
- Around line 102-106: The stem-separation step currently swallows all
exceptions and never uses the produced stems (stems is only logged), so
separation can fail silently or be wasted on success; update the flow so that
AudioStemSeparator().separate_audio(...) returns are passed into the downstream
call (e.g., include stems in the payload or call run_analysis_job(request,
stems=stems)) when successful, and do not convert all errors to warnings — catch
only expected exceptions or log the full error and propagate/fail the job (raise
or return an error status) instead of continuing as success; update the
try/except around separate_audio to call run_analysis_job with the stems
variable on success and to surface failures (or use a controlled mock fallback
with an explicit flag) rather than a silent logging.warning.

In
`@services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py`:
- Around line 38-46: 현재 _load_model 호출은
demucs.pretrained.get_model(self.model_name)를 직접 사용해 런타임에 원격 다운로드와 무결성 검증을
우회하므로, 로컬에 사전 프로비저닝된 weight와 체크섬을 우선 검증한 후 없으면 명시적으로 실패하도록 수정하세요: update
_load_model to first resolve a project-local artifact path for self.model_name
(or a configured model directory), verify the file exists and validate its
checksum/signature, only then load the model (and call .eval()); remove or gate
any fallback to demucs.pretrained.get_model that would perform a network fetch
so that cold-cache runs fail fast with a clear error rather than downloading
remotely.

In `@services/analysis-engine/tests/test_audio_separator.py`:
- Around line 42-64: The test currently only verifies apply_model was called but
not that it received the OOM-mitigation parameters; update the test in
test_audio_separator.py to assert apply_model was called with the expected
kwargs by inspecting mock_apply_model.call_args.kwargs after calling
separator.separate_audio (use the existing mock_apply_model and
separator.separate_audio/segment_seconds), and assert kwargs["split"] is True,
kwargs["segment"] == 2.0 (or segment_seconds), kwargs["overlap"] == 0.25,
kwargs["shifts"] == 1, and kwargs["progress"] is False; keep the existing checks
for call counts and returned stems.

In `@services/analysis-engine/tests/test_cli.py`:
- Around line 377-443: The test
test_cli_main_temporal_analyzer_and_separator_mock_success currently only checks
jobId and can pass without calling librosa.load or
AudioStemSeparator.separate_audio; update the test to assert those functions are
actually invoked with expected args: replace or wrap the current monkeypatch for
librosa.load and
bandscope_analysis.separation.audio_separator.AudioStemSeparator with
spies/mocks that record call counts and parameters, then after cli.main() assert
librosa.load was called with the expected path/sr/mono/duration and that
AudioStemSeparator.separate_audio was called with the expected audio,
sample_rate and segment_seconds; also add a complementary test (or extend this
one) exercising the --separate-stems gate to confirm separation is skipped when
the flag is absent and that separate_audio is not called.

In `@supply-chain/supplemental-component-inventory.json`:
- Around line 14-23: The htdemucs modelArtifacts entry is missing checksum
verification data; add a checksum field (and algorithm if your schema supports
it, e.g., checksumAlgorithm or checksumType) to the htdemucs object so releases
can validate cached weights; locate the "modelArtifacts" array and the entry
with "name": "htdemucs" (and "version": "demucs-v4.0", "sourceUrl": "...") and
include the checksum value and algorithm consistent with other artifacts in this
inventory.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d034c4c0-df4d-4382-ac78-7aa825c1dea5

📥 Commits

Reviewing files that changed from the base of the PR and between f6cbd3b and e9e800e.

⛔ Files ignored due to path filters (1)
  • services/analysis-engine/uv.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • .github/workflows/build-baseline.yml
  • services/analysis-engine/pyproject.toml
  • services/analysis-engine/src/bandscope_analysis/cli.py
  • services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py
  • services/analysis-engine/tests/test_audio_separator.py
  • services/analysis-engine/tests/test_cli.py
  • supply-chain/supplemental-component-inventory.json

Comment thread .github/workflows/build-baseline.yml
Comment thread services/analysis-engine/src/bandscope_analysis/cli.py Outdated
Comment thread services/analysis-engine/src/bandscope_analysis/cli.py Outdated
Comment thread services/analysis-engine/tests/test_audio_separator.py Outdated
Comment thread services/analysis-engine/tests/test_cli.py
Comment thread supply-chain/supplemental-component-inventory.json
@seonghobae seonghobae enabled auto-merge June 10, 2026 13:53
@seonghobae

Copy link
Copy Markdown
Collaborator Author

@coderabbitai resolve

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 10, 2026
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Comments resolved and changes approved.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@coderabbitai resolve

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 10, 2026
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Comments resolved and changes approved.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@coderabbitai resolve

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Rate Limit Exceeded

@seonghobae have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 0 seconds before sending another message.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

Updated #111 to fix the failing checks after the scheduler/develop merge.

What changed:

  • Restored scheduler artifacts from current develop so this PR no longer deletes them.
  • Resolved the Python dependency set for stem separation as a compatible Demucs/Torch/Torchaudio stack: numpy<2.0, torch<=2.2.2, torchaudio<=2.2.2, while keeping urllib3>=2.7.0 and Pygments 2.20.0 in the lockfile.
  • Avoided importing real Demucs/Torchaudio in unit tests by injecting fake demucs.* modules.
  • Added coverage for missing model weights, checksum mismatch, invalid --separate-stems input, and temporal-probe failure.

Validation:

  • python3 scripts/checks/verify_supply_chain.py
  • uv run ruff check src tests
  • uv run ruff format --check src tests
  • uv run mypy src
  • uv run pytest tests --cov=src/bandscope_analysis --cov-report=term-missing --cov-fail-under=100 (282 passed, 100% coverage)
  • ./scripts/harness/quickcheck.sh (passed after documented npm install; npm warned local Node is v24 while repo asks for >=22 <23)
  • git diff --check

Security Notes:

  • Stem-separation model weights remain pre-provisioned local artifacts; no runtime model download path was added.
  • Local model loading now has explicit tests for missing weights and checksum mismatch fail-closed behavior.
  • Dependency constraints avoid the broken NumPy 2.x/Torch/Torchaudio combination seen in CI while keeping the vulnerability-gated lock entries current.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@coderabbitai approve

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 11, 2026
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Comments resolved and changes approved.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

Correction to the previous validation note: the final pushed dependency stack is torch>=2.11.0,<2.12.0 and torchaudio>=2.11.0,<2.12.0, not Torch 2.2.x. I changed this after dependency-review and Trivy flagged GHSA-53q9-r3pm-6pq6 against Torch 2.2.2.

Additional validation after the final lockfile:

  • uv run --project services/analysis-engine python -c "import torch, torchaudio; print(torch.__version__); print(torchaudio.__version__)" -> 2.11.0 / 2.11.0
  • uv sync --project services/analysis-engine --group dev --frozen
  • uv run --project services/analysis-engine --with pip-audit==2.8.0 pip-audit --local --strict -> no known vulnerabilities
  • ./scripts/harness/quickcheck.sh -> passed

Security Notes update:

  • The vulnerable Torch 2.2.2 lock entry was removed.
  • Torch and Torchaudio are pinned to the same patched minor line to avoid ABI mismatch while satisfying the PyTorch advisory patched-version requirement.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@seonghobae

Copy link
Copy Markdown
Collaborator Author

@coderabbitai approve

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Comments resolved and changes approved.

@seonghobae

Copy link
Copy Markdown
Collaborator Author

Closing this PR instead of merging it.

Current live state: #111 is approved but DIRTY, and its branch diff against develop is no longer a narrow stem-separation change. It touches 55 files, including workflow/action-pin files, frontend package/test files, section utilities, YouTube/security tests, supply-chain verification tests, and several analysis modules that have already been updated by newer merged PRs.

Concrete blockers:

  • The branch would roll back current develop work, including .github/workflows/* pin/hardening changes, scripts/checks/verify_supply_chain.py, services/analysis-engine/src/bandscope_analysis/sections/utils.py, YouTube tests/security behavior, GrooveMap updates, and other merged analysis/test changes.
  • It adds new direct runtime dependencies demucs, torch, and torchaudio with a very large transitive footprint, while also downgrading the current numba constraint. That needs current dependency-admission evidence under the BandScope dependency policy before it can ship.
  • The model artifact inventory only records an 8-character checksum prefix (f7e0c4bc) for htdemucs and points at ~/.cache/torch/hub/checkpoints/. Model artifacts need full provenance/checksum and release inventory evidence, not a user-home cache path plus a prefix check.
  • The feature crosses the local model/file-processing trust boundary, so it needs a fresh app-security review for untrusted audio input, model artifact loading, safe failure, logging/privacy impact, and tests.
  • The PR is stale enough that safely merging would require rebuilding the change on top of current develop, not resolving conflicts in place.

Please reopen as a fresh, focused PR if stem separation is still desired: start from current develop, keep the dependency/model admission in the same PR, use full artifact checksums, and avoid rolling back unrelated security/workflow/test changes.

@seonghobae seonghobae closed this Jun 12, 2026
auto-merge was automatically disabled June 12, 2026 05:14

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Epic: Spectral Analysis & Source Separation (Stems)

1 participant